-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add Json schema compilation in smoke test #109
Conversation
…ssful run and framing for demonstrating any current functional lapses -
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are commits in this PR also in other outstanding PRs?
I need to be walked through this ... @wandmagic I can see a file adding to smoke test logic but do we want to accept the other commits here?
sure i can cherry pick them, i still need to actually use the AJV to do a new separate test instead of adding it to an existing one |
Cool thanks. There is more being added to my working branch so this will have to be clean. Happy to discuss. |
@@ -36,6 +36,9 @@ | |||
<flag ref="id"/> | |||
<model> | |||
<field ref="field-1only"/> | |||
<field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I brought this up in #111, but is there some intention we can explain there or here per https://github.com/usnistgov/metaschema-xslt/pull/111/files#r1524912715. It now seems 111 is only a dupe with this change but none of this substantive work in this PR, is there a reason for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was basically a requirement to get the schema to compile.
without this change we get two fields that try and find a JSON schema definition that doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so this is needed to fix a test we definitely have a bug on ours hands.
Is this #109 PR connected to a specific issue yet? Just got back from leave and I am talking to Wendell about this on Element/Matrix/Gitter/what-have-you, so I am trying to make sure I understand the context given the other (very good) changes in this PR, but this is basically invalid Metaschema so it caught my attention immediately. If we were purposefully testing "bad model behavior" and negative testing with this model I would let it go, but it seems that was not the intent of this model and behavior and we should identify it and move that out into a bug. Or be very lazy and fix it here, that just causes more confusing git commit reviews later on.
@wendellpiez, thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the scope of the PR, I am with AJ. If there is a bug in a metaschema that prohibits generating a JSON Schema, that is not a problem for this Issue but for another one, where we can bring the right attention to bear.
This doesn't mean the problem isn't real, just that we can't cut corners. Demonstration please. :-)
Boy would I love to be able to push a button and have the robots take my schema for a run -- I have that for XSD, can anyone help me with JSON Schema? See #110. Let's do that and determining whether metaschema X makes good JSON Schema will something we can see, not just argue about.
Meanwhile thanks to you both -- @wandmagic please do not hesitate to ask for assistance or further elucidation --
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the complexity of the system is such that both formal validation (against its own schema) and 'validation in operation' (i.e. try and see what happens) must be conducted. This is because although formal validation goes a long way to ensuring the correctness of the outputs wrt 'intent' (correctness defined by Metaschema semantics), it can't go the full distance.
To deploy and fully test a Metaschema (module) typically entails:
- Formal validation (against the Metaschema XSD schema) of all Metaschema modules under 'import'
- Schematron validation of top-level metaschem module to check imports
- Run the Metaschema to produce derivative artifacts
- Including XSD, JSON Schema, InspectorXSLT, oscal-cli version, converter XSLTs, documentation stacks, indexes, you name it
- Unit test and field test any and all of these
We have been doing all this all along, albeit without much automation to help.
Indeed, that is essentially the problem we now have, that without being able both to record and document these processes, and automate testing to be supervised by Metaschema maintainers who are not developers -- we are essentially at a place where the knowledge of how it is done cannot be communicated. (Right, @iMichaela?)
The way I have been trying to address this is by starting to add tests, scripts and readmes in whatever application I need to be working on - recently JSON Schema (#108) and also InspectorXSLT (#98).
Oh look, there is #102 -- which I just merged. It contains work that could be further integrated into CI/CD as well -- I hope this helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment on my comment - note that the stack of tests/validations listed is a funnel. The rules enforced by the Metaschema XSD are very tight. But they are also assumed to be true (to be in force) by the Schematron at the next level down. This means the Schematron is much simpler than it would be otherwise, as it can be built with the XSD rules as "prior knowledge". Same with the next level. Because the Schematron has checked that imports work, the next level doesn't have to fail safe on that. Detecting and preventing the problems at the top of the stack is much easier, faster and easier to tailor than dealing with them at the bottom, where they are typically seen as "garbage out".
All this is in accordance with the 'functional' and 'declarative' mindset cultivated by XML, a descendant of LISP.
The stack is designed so that when a data instance (a Metaschema) passes both XSD and Schematron validation, everything below will be deterministic and 'just work'. So that any problems in unit testing the final XSDs and JSON Schemas etc can be treated as processor bugs to be reported back, and Metaschema authors do not have to be on the hook for processing errors (i.e. jump in and turn into developers).
Committer Notes
Fix JSON Schema validation issues
All Submissions:
Changes to Core Features: